-
Notifications
You must be signed in to change notification settings - Fork 372
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Telemetry component cleanup #4742
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jnummelin
previously approved these changes
Jul 12, 2024
This pull request has merge conflicts that need to be resolved. |
There are quite a few assignments in these methods that are lost if the receiver is not a pointer. Signed-off-by: Tom Wieczorek <twieczorek@mirantis.com>
Don't store them in the component directly. Signed-off-by: Tom Wieczorek <twieczorek@mirantis.com>
This was basically a copy of the analytics.Client interface. Signed-off-by: Tom Wieczorek <twieczorek@mirantis.com>
That way, the componend isn't even registered in case telemetry has not been enabled during compilation time. Signed-off-by: Tom Wieczorek <twieczorek@mirantis.com>
Handle its lifecycle directly in the telemetry goroutine. Signed-off-by: Tom Wieczorek <twieczorek@mirantis.com>
The extra field is not providing any extra value at the moment. Signed-off-by: Tom Wieczorek <twieczorek@mirantis.com>
The storage type is part of the node config and cannot be reconciled. Don't take it from the cluster config, but let it be set directly into the componend via a public fied. This allows for removing the cluster config from the struct. Signed-off-by: Tom Wieczorek <twieczorek@mirantis.com>
The Start/Stop methods and the Reconcile method may be called concurrently by different goroutines. Ensure that there's no races when starting and stopping the telemetry loop. Also replace the bespoke for loop with wait.UntilWithContext, which is tailor-made for that purpose. Signed-off-by: Tom Wieczorek <twieczorek@mirantis.com>
twz123
force-pushed
the
telemetry-cleanup
branch
from
July 12, 2024 16:37
839440b
to
2def786
Compare
ncopa
approved these changes
Jul 23, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
analyticsClient
abstraction, which was basically a copy of theanalytics.Client
interface.wait.UntilWithContext
, which is tailor-made for that purpose.Type of change
How Has This Been Tested?
Checklist: